Skip to content

Fix unboxing of numpy boolean scalars in C-backend#1790

Merged
ricardoV94 merged 1 commit intopymc-devs:mainfrom
ricardoV94:fix_c_scalar_bool_unbox
Jan 5, 2026
Merged

Fix unboxing of numpy boolean scalars in C-backend#1790
ricardoV94 merged 1 commit intopymc-devs:mainfrom
ricardoV94:fix_c_scalar_bool_unbox

Conversation

@ricardoV94
Copy link
Copy Markdown
Member

@ricardoV94 ricardoV94 commented Dec 15, 2025

We were seeing weird things where an output of ScalarFromTensor for a np.array(True), would evaluate to False. See this old comment here:

# TODO: Why is .item() needed?
mode: Literal["full", "valid", "same"] = "full" if full_mode.item() else "valid"
outputs[0][0] = scipy_convolve(in1, in2, mode=mode, method=self.method)

And this snippet would fail:

import numpy as np
import pytensor
import pytensor.tensor as pt

x = pt.scalar("x", dtype="bool")
y = pt.scalar_from_tensor(x)
fn = pytensor.function([x], y)
assert fn(np.array(True, dtype=bool))   # AssertionError :O

I tracked it down to the c_sync method for ScalarType, which is called when the C code has to generate a Python object as an output (or intermediate computation if going into python mode). Basically unboxing of the internal representation of scalars into numpy scalar arrays.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the C-backend where numpy boolean scalars were not being properly unboxed by the ScalarFromTensor operation. The issue caused np.array(True) to evaluate as False after going through scalar_from_tensor, requiring a workaround using .item() in some places like conv.py.

Key changes:

  • Replaced deprecated PyArrayScalar_New and PyArrayScalar_ASSIGN APIs with PyArray_Scalar in the C-backend scalar synchronization code
  • Removed the .item() workaround from the convolution operation that is no longer needed
  • Added test coverage for boolean scalar conversion to prevent regression

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
pytensor/scalar/basic.py Refactored c_sync method to use PyArray_Scalar with proper descriptor handling instead of deprecated PyArrayScalar_* APIs, fixing boolean scalar unboxing. Incremented C code cache version.
pytensor/tensor/signal/conv.py Removed .item() workaround from Convolve2d.perform method that is no longer needed with the C-backend fix.
tests/tensor/test_basic.py Added test_bool_scalar_from_tensor to verify boolean scalars are correctly converted through scalar_from_tensor without losing their truth value.

Comment thread pytensor/tensor/signal/conv.py
@ricardoV94 ricardoV94 changed the title Fix unboxing of numpy boolean Scalars in C-backend Fix unboxing of numpy boolean scalars in C-backend Dec 15, 2025
@ricardoV94
Copy link
Copy Markdown
Member Author

ricardoV94 commented Dec 15, 2025

Failing test is a precision issue with the new conv2D tests that were merged without a seed, and having too strict tolerance. That's fixed by #811

@jessegrabowski jessegrabowski force-pushed the fix_c_scalar_bool_unbox branch from 6b1c93e to e88a658 Compare January 5, 2026 00:31
@ricardoV94 ricardoV94 force-pushed the fix_c_scalar_bool_unbox branch from e88a658 to 97d0afe Compare January 5, 2026 12:00
def perform(self, node, inputs, outputs):
in1, in2, full_mode = inputs

if isinstance(full_mode, np.bool):
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you accidentally force-pushed away this change. Re-added it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry

@ricardoV94 ricardoV94 merged commit 5062d3e into pymc-devs:main Jan 5, 2026
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working C-backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants